Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(impls): add rust_decimal::Decimal impl #273

Closed
wants to merge 1 commit into from

Conversation

seanpianka
Copy link

@seanpianka seanpianka commented Mar 19, 2024

Goal

Provide a serde-based implementation of TS for rust_decimal::Decimal
Closes #272

Changes

Derive an implementation for rust_decimal::Decimal using feature flag serde-with-str.

Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made.

@escritorio-gustavo
Copy link
Contributor

escritorio-gustavo commented Mar 19, 2024

Hey, thanks for the PR! Please check out the Contributing guide for instructions on how to update the README.

Edit: I see the link in the PR template is incorrectly pointing at the CHANGELOG file instead of CONTRIBUTING, sorry about that, it's already been fixed, both in the template and in this PR's description

@NyxCode
Copy link
Collaborator

NyxCode commented Mar 19, 2024

I'm a bit confused by rust-decimal in general. Their usage of features seems a bit odd to me (though we're abusing them similarly I guess).

If we want support rust-decimal going forward, I think we need to seamlessly support all these modes - and I'm not sure that's easily possible.

Re-exposing all features of rust_decimal, for example, doesn't seem great.
rust_decimal already has feature gates for a wide variety of libraries, so maybe it'd be worth a shot to open a PR there. In the rust_decimal crate, this could be implemented nicely using #[cfg(feature = "..")] annotations.

@NyxCode
Copy link
Collaborator

NyxCode commented May 17, 2024

Closing this for now. I think we established that we can't support this properly.
Either rust_decimal implements TS behind a feature-flag, or users will have to use #[ts(as = ..)] or #[ts(type = "..")].

@NyxCode NyxCode closed this May 17, 2024
@NyxCode NyxCode mentioned this pull request Sep 7, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: rust_decimal::Decimal TS implementation
3 participants